Fix non-working colorscheme plugins #3761
Conversation
BTW @JoeKar you might have some thoughts about this, since this is directly related to your #2836 (although it didn't change the behavior of |
|
Regarding
Are you implying that If so, I don't think that is the case, I am on master.
Sorry, forget what I said. I totally misread the whole thing as syntax file instead. |
micro/internal/config/plugin.go Lines 77 to 83 in cd0dc9a Indeed, I missed that as well as updating the documentation. Should we fix this here too? I assume it will finally break https://github.com/KiranWells/micro-nord-tc-colors till it is adapted accordingly (move |
JoeKar
left a comment
There was a problem hiding this comment.
A feasible workaround so far.
Maybe we find something better in the future.
What about IsLoaded()...do we change it with this PR?
I haven't thought what to do with it. Anyway, we don't need to do anything with it in this PR. |
Adding early validation of options in ReadSettings() caused a regression: colorschemes registered by plugins via config.AddRuntimeFile() stopped working, since ReadSettings() is called when plugins are not initialized (or even loaded) yet, so a colorscheme is not registered yet and thus its validation fails. Fix that with an ad-hoc fix: treat the "colorscheme" option as a special case and do not verify it early when reading settings.json, postponing that until the moment when we try to load this colorscheme.
Micro "allows" plugins to register colorschemes via config.AddRuntimeFile(). However, that has never really worked, since InitColorscheme() is called earlier than plugins init() or even preinit() callbacks are called. To work around that, plugins that use it (e.g. nord-tc [1]) are using a tricky hack: call config.AddRuntimeFile() not in init() or preinit() but directly in Lua's global scope, so that it is called earlier, when the plugin's Lua code is loaded. This hack is not guaranteed to work, and works by chance. Furthermore, it only works when starting micro, and doesn't work after the `reload` command. (The reason it doesn't work is that PluginAddRuntimeFile() calls FindPlugin() which calls IsLoaded() which returns false, since, well, the plugin is not loaded, it is only being loaded. And the reason why it works when starting micro is that in that case IsLoaded() confusingly returns true, since GlobalSettings[p.Name] has not been set yet.) So move InitColorscheme() call after calling plugins init/preinit/ postinit callbacks, to let plugins successfully register colorschemes in any of those callbacks instead of using the aforementioned hack. [1] https://github.com/KiranWells/micro-nord-tc-colors
fe714b1 to
e923640
Compare
Fixes #3455
Micro "allows" plugins to register colorschemes via
config.AddRuntimeFile(). However, that has never really worked, sinceInitColorscheme()is called earlier than pluginsinit()or evenpreinit()callbacks are called.To work around that, colorscheme plugins (e.g. https://github.com/KiranWells/micro-nord-tc-colors) are using a tricky hack: call
config.AddRuntimeFile()not ininit()orpreinit()but directly in Lua's global scope, so that it is called even earlier, when the plugin's Lua code is loaded. This hack is not guaranteed to work, and works by chance. Furthermore, it only works when starting micro, and doesn't work after thereloadcommand. (The reason it doesn't work is thatPluginAddRuntimeFile()callsFindPlugin()which callsIsLoaded()which returns false, since, well, the plugin is not loaded, it is only being loaded. And the reason why it works when starting micro is that in that caseIsLoaded()confusingly returns true, sinceGlobalSettings[p.Name]has not been set yet.)Moreover, after PR #3343 even this hack stopped working, since we added validation of option values (including the
colorschemeoption) when reading them from settings.json, which happens even earlier than loading plugins. So validation checks if the colorscheme exists, and it doesn't exist yet, since it has not been registered by the plugin yet.So, this PR does 2 things:
colorschemeoption only (treating this option as a special case). We "validate" it later: when we try to load this colorscheme and fail, we reset it to the default colorscheme.This fix is quite ad-doc and a bit hacky, so any better suggestions are welcome.
init(),preinit()andpostinit()callbacks), so that plugins can finally successfully register colorschemes in those callbacks, instead of using the above tricky hack which is not guaranteed to work.